Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Chore/feeds test suite cleanup #15215

Merged
merged 6 commits into from
Nov 13, 2024
Merged

Chore/feeds test suite cleanup #15215

merged 6 commits into from
Nov 13, 2024

Conversation

Oozyx
Copy link

@Oozyx Oozyx commented Nov 12, 2024

Requires

Supports

@Oozyx Oozyx requested a review from eduard-cl November 12, 2024 21:52
@Oozyx Oozyx requested a review from a team as a code owner November 12, 2024 21:52
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 12, 2024 •
edited
Loading

AER Report: Operator UI CI

aer_workflow , commit , Breaking Changes GQL Check

1. Workflow failed to complete successfully:[convictional/trigger-workflow-and-wait@f69fa9e]

Source of Error: 
2024-11-13T15:33:15.5819313Z Checking conclusion [failure]
2024-11-13T15:33:15.5820223Z Checking status [completed]
2024-11-13T15:33:15.5825668Z Conclusion is not success, it's [failure].
2024-11-13T15:33:15.5826567Z Propagating failure to upstream job

Why: The triggered workflow did not complete successfully. The conclusion status was failure instead of success, causing the upstream job to propagate the failure.

Suggested fix: Investigate the logs of the triggered workflow at https://github.com/smartcontractkit/operator-ui/actions/runs/11820519907 to identify the specific cause of failure and address it accordingly.

AER Report: CI Core ran successfully ✅

aer_workflow , commit

eduard-cl
eduard-cl reviewed Nov 13, 2024
View reviewed changes
Copy link
Contributor

@eduard-cl eduard-cl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, left some small comments

contracts/src/v0.8/feeds/test/DualAggregator.t.sol Outdated
}

function test_RevertIf_CalldataLengthMismatch() public {
vm.startPrank(transmitters[0]);
vm.startPrank(s_transmitters[0]);
vm.expectRevert(DualAggregator.CalldataLengthMismatch.selector);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to what I said in the other comments, following the solidity guide:

There should be no code between vm.expectEmit/vm.expectRevert and the function call.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here you're correct, we should place expectRevert above the function call

@Oozyx Oozyx requested a review from eduard-cl November 13, 2024 15:32
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@Oozyx Oozyx merged commit 33b43cf into feeds-project-develop Nov 13, 2024
102 of 105 checks passed
@Oozyx Oozyx deleted the chore/feeds-test-suite-cleanup branch November 13, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eduard-cl eduard-cl eduard-cl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Oozyx @eduard-cl